Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

add lesson05 for okhttp extra credit #50

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

taobaorun
Copy link

add okhttp extra credit

Copy link
Owner

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good. A few minor comments.

java/src/main/java/lesson05/README.md is not needed, right?

java/src/main/java/lesson05/solution/Hello.java Outdated Show resolved Hide resolved
java/src/main/java/lesson05/solution/Hello.java Outdated Show resolved Hide resolved
@@ -0,0 +1,23 @@
# Lesson 5 - Using Existing Open Source Instrumentation
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to call this 'lesson05'. In all tutorials this is called 'extracredit, and there already exists a package for that, with a readme: https://github.com/yurishkuro/opentracing-tutorial/tree/master/java/src/main/java/extracredit

You can add everything there.


@GET
public String format(@QueryParam("helloStr") String helloStr, @Context HttpHeaders httpHeaders) {
try (Scope scope = Tracing.startServerSpan(tracer, httpHeaders, "publish")) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you really want to implement the fill solution for the extra credit, then this should also use the opentracing-contrib library for Dropwizard. The point of extracredit is to not do instrumentation manually.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants